Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Fix UI Actions context menu positioning for the Anomaly Swim Lane #71839

Merged
merged 2 commits into from
Jul 15, 2020

Conversation

darnautov
Copy link
Contributor

Summary

#70705 has changed the logic of positioning the UI context menu, it relies on the element that has received a click event originally. But the anomaly swim lane was completely rerendered on cell selection, which caused unmounting the original element that received the click from the DOM tree. I've tried to resolve it with minimum changes by caching selection in the component state and checking it with shouldComponentUpdate lifecycle hook.

Please bear in mind on the Anomaly Explorer page the swim lane still rerenders on the selection change, it happens because of the way state works.

Checklist

@darnautov darnautov added bug Fixes for quality problems that affect the customer experience :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 15, 2020
@darnautov darnautov requested a review from a team as a code owner July 15, 2020 10:16
@darnautov darnautov self-assigned this Jul 15, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@darnautov darnautov merged commit 4c654c4 into elastic:master Jul 15, 2020
@darnautov darnautov deleted the ML-fix-swim-lane-render branch July 15, 2020 13:07
darnautov added a commit to darnautov/kibana that referenced this pull request Jul 15, 2020
elastic#71839)

* [ML] fix swim lane embeddable rerenders

* [ML] fix TS
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 15, 2020
* master:
  [Form lib] Memoize form hook object and fix hook array deps (elastic#71237)
  [uiActions] Support emitting nested triggers and actions (elastic#70602)
  add short sleep before clicking Remove on sample data (elastic#71104)
  Fixed the beta badge layout. (elastic#71835)
  Restores task for downloading Chromium builds (elastic#71749)
  [logging] Format new platform json logging to ECS (elastic#71138)
  add policy details and update SO limit requests (elastic#71789)
  Convert vis_type_vega to Typescript (elastic#68915)
  [ML] Fix UI Actions context menu positioning for the Anomaly Swim Lane (elastic#71839)
darnautov added a commit to darnautov/kibana that referenced this pull request Jul 16, 2020
elastic#71839)

* [ML] fix swim lane embeddable rerenders

* [ML] fix TS
darnautov added a commit that referenced this pull request Jul 16, 2020
#71839) (#72018)

* [ML] fix swim lane embeddable rerenders

* [ML] fix TS
darnautov added a commit that referenced this pull request Jul 16, 2020
#71839) (#71862)

* [ML] fix swim lane embeddable rerenders

* [ML] fix TS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience :ml release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants